Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: calculate network id #212

Merged
merged 9 commits into from
Dec 5, 2024
Merged

Conversation

ToniRamirezM
Copy link
Contributor

Description

Calculate network ID instead of reading it from config

@ToniRamirezM ToniRamirezM self-assigned this Nov 29, 2024
cmd/run.go Outdated
reorgDetectorL2 *reorgdetector.ReorgDetector,
l2Client *ethclient.Client,
) *bridgesync.BridgeSync {
if !isNeeded([]string{cdkcommon.RPC, cdkcommon.AGGSENDER}, components) {
return nil
}

ethermanClient, err := newEtherman(c)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of passing c config.Config to be able to create a ethClient, inject the etherClient. There are 2 functions on run.go that create the common instance for all classes:

  • runL1ClientIfNeeded
  • runL2ClientIfNeeded

cmd/run.go Outdated
@@ -77,8 +77,8 @@ func start(cliCtx *cli.Context) error {

l1InfoTreeSync := runL1InfoTreeSyncerIfNeeded(cliCtx.Context, components, *c, l1Client, reorgDetectorL1)
claimSponsor := runClaimSponsorIfNeeded(cliCtx.Context, components, l2Client, c.ClaimSponsor)
l1BridgeSync := runBridgeSyncL1IfNeeded(cliCtx.Context, components, c.BridgeL1Sync, reorgDetectorL1, l1Client)
l2BridgeSync := runBridgeSyncL2IfNeeded(cliCtx.Context, components, c.BridgeL2Sync, reorgDetectorL2, l2Client)
l1BridgeSync := runBridgeSyncL1IfNeeded(cliCtx.Context, components, *c, reorgDetectorL1, l1Client)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know that is not related to your PR, but renaming c for config or even for cfg would be really nice

Copy link
Contributor

@vcastellm vcastellm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Collaborator

@Stefan-Ethernal Stefan-Ethernal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generally LGTM, left a minor comments.

etherman/etherman.go Outdated Show resolved Hide resolved
etherman/etherman.go Outdated Show resolved Hide resolved
etherman/etherman.go Outdated Show resolved Hide resolved
ToniRamirezM and others added 3 commits December 5, 2024 11:05
Co-authored-by: Stefan Negovanović <[email protected]>
Co-authored-by: Stefan Negovanović <[email protected]>
Co-authored-by: Stefan Negovanović <[email protected]>
Copy link

sonarcloud bot commented Dec 5, 2024

Please retry analysis of this Pull-Request directly on SonarQube Cloud

@ToniRamirezM ToniRamirezM merged commit 26a26d1 into release/v0.5.0 Dec 5, 2024
12 checks passed
@ToniRamirezM ToniRamirezM deleted the feature/networkID branch December 5, 2024 11:07
ToniRamirezM added a commit that referenced this pull request Dec 5, 2024
* feat: calculate network id

* feat: refactor

* feat: refactor

* feat: separeted function to obtain the rollupID

* feat: obtain RollupID without etherman instance

* Update etherman/etherman.go

Co-authored-by: Stefan Negovanović <[email protected]>

* Update etherman/etherman.go

Co-authored-by: Stefan Negovanović <[email protected]>

* Update etherman/etherman.go

Co-authored-by: Stefan Negovanović <[email protected]>

---------

Co-authored-by: joanestebanr <[email protected]>
Co-authored-by: Stefan Negovanović <[email protected]>
Stefan-Ethernal added a commit that referenced this pull request Dec 9, 2024
* feat: calculate network id (#212)

* feat: calculate network id

* feat: refactor

* feat: refactor

* feat: separeted function to obtain the rollupID

* feat: obtain RollupID without etherman instance

* Update etherman/etherman.go

Co-authored-by: Stefan Negovanović <[email protected]>

* Update etherman/etherman.go

Co-authored-by: Stefan Negovanović <[email protected]>

* Update etherman/etherman.go

Co-authored-by: Stefan Negovanović <[email protected]>

---------

Co-authored-by: joanestebanr <[email protected]>
Co-authored-by: Stefan Negovanović <[email protected]>

* fix: typo

---------

Co-authored-by: joanestebanr <[email protected]>
Co-authored-by: Stefan Negovanović <[email protected]>
Co-authored-by: Stefan Negovanović <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants